Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use LUT for linear to sRGB conversion #18

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

thvdveld
Copy link
Contributor

Using a LUT for the linear to sRGB conversion improves decoding performance for the decode_into benchmark:

decode_into LGF5]+Yk^6#M@-5c,1J5@[or[Q6./200
    time:   [766.58 µs 769.86 µs 776.05 µs]
    change: [-62.967% -62.784% -62.602%] (p = 0.00 < 0.05)
    Performance has improved.

However, the size of the LUT is 8.192 u8. The fast-linear-to-srgb feature flag was added, which is enabled by default. In cases where binary size is more important than performance, the feature can be disabled.

Another note in the implementation: using a lookup table reduces the accuracy of the conversion, compared to the original implementation. But all current tests pass. Higher accuracy can be achieved by increasing the LUT size.

@thvdveld
Copy link
Contributor Author

Oh, I didn't see that there were more tests.

@thvdveld
Copy link
Contributor Author

thvdveld commented Jul 15, 2024

This test fails even on main on my machine:

 ---- tests::decode_blurhash_pixbuf stdout ----
thread 'tests::decode_blurhash_pixbuf' panicked at src/lib.rs:455:9:
assertion `left == right` failed
  left: [166, 198, 128, 222, 221]
 right: [77, 210, 4, 80, 15]

@rubdos
Copy link
Member

rubdos commented Jul 15, 2024

This test fails even on main on my machine:

That might not be an issue; that test has failed randomly anyway. Can you maybe check the visuals for decoding? cargo test should yield you data/out.png, you can compare it to the version that's checked in in the repo. We need to clear that behaviour at some point... cargo test should overwrite files in the repo :'-)

@rubdos
Copy link
Member

rubdos commented Jul 15, 2024

Cross-checked with #14, doesn't fail.

@rubdos
Copy link
Member

rubdos commented Jul 15, 2024

Would this be a case for https://docs.rs/phf/latest/phf/ ? 🤔

@rubdos
Copy link
Member

rubdos commented Jul 22, 2024

@thvdveld a rebase on main should make the tests pass with a higher probability ;-)

Using a LUT for the linear to sRGB conversion improves decoding
performance for the decode_into benchmark:

```
decode_into LGF5]+Yk^6#M@-5c,1J5@[or[Q6./200
    time:   [766.58 µs 769.86 µs 776.05 µs]
    change: [-62.967% -62.784% -62.602%] (p = 0.00 < 0.05)
    Performance has improved.
```

However, the size of the LUT is 8.192 u8. The `fast-linear-to-srgb`
feature flag was added, which is enabled by default. In cases where
binary size is more important than performance, the feature can be
disabled.

Another note in the implementation: using a lookup table reduces the
accuracy of the conversion, compared to the original implementation.
But all current tests pass. Higher accuracy can be achieved by
increasing the LUT size.
@thvdveld
Copy link
Contributor Author

All checks have passed! 🎉

@rubdos rubdos requested a review from gferon July 22, 2024 15:00
@rubdos
Copy link
Member

rubdos commented Jul 23, 2024

On my armv7 phone:

decode_into LGF5]+Yk^6#M@-5c,1J5@[or[Q6./200
                        time:   [4.3007 ms 4.3054 ms 4.3104 ms]
                        change: [-70.320% -70.238% -70.170%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

That's 10ms faster :'-)

build.rs Show resolved Hide resolved
@rubdos rubdos merged commit 580454a into whisperfish:main Jul 23, 2024
5 checks passed
@thvdveld thvdveld deleted the faster-decode branch July 23, 2024 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants